Skip to content

Conversation

@jscheffl
Copy link
Contributor

Add the next increment of feature:

Adding a button to change maintenance comments for Edge Workers:
image

After change:
image

FYI @dheerajturaga

@boring-cyborg boring-cyborg bot added area:providers provider:edge Edge Executor / Worker (AIP-69) / edge3 labels Sep 11, 2025
@dheerajturaga
Copy link
Member

Nice! Functionality works! a few cleanup suggestions

  1. When we request maintenance, we should probably not show up the "update maintenance comment" button until the worker enters maintenance mode. In the image below, it does not allow me to update comment when its still in maintenance request state
image
  1. it would be cleaner if clicking on the "Update Comments" dialogue box actually closes the box. Currently you need to manually hit the close button
image

Copy link
Member

@dheerajturaga dheerajturaga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving as base functionality works, suggestions are nits which can be addressed in a follow up

@jscheffl
Copy link
Contributor Author

Nice! Functionality works! a few cleanup suggestions

1. When we request maintenance, we should probably not show up the "update maintenance comment" button until the worker enters maintenance mode. In the image below, it does not allow me to update comment when its still in maintenance request state

2. it would be cleaner if clicking on the "Update Comments" dialogue box actually closes the box. Currently you need to manually hit the close button

Thanks for the feedback. (1) is actually a feature and was as far as I remember in the previous UI the same. But can double-check.

(2) is a blocker for me, I am not sure why this happens but must be resolved prior merge. Funny it actually updates but does not close. It is the "same" code like in other dialogs but don't understand why it is different here :-D

@dheerajturaga
Copy link
Member

@jscheffl for 1. I was suggesting we remove the button altogether, no need to have it until the worker enters Maintenance mode

@jscheffl jscheffl force-pushed the feature/add-maintenance-comment-change-to-edge-react-ui branch from 2ff044a to bbe5ac8 Compare September 12, 2025 19:33
@jscheffl
Copy link
Contributor Author

  1. When we request maintenance, we should probably not show up the "update maintenance comment" button until the worker enters maintenance mode. In the image below, it does not allow me to update comment when its still in maintenance request state

Thanks for the feedback. (1) is actually a feature and was as far as I remember in the previous UI the same. But can double-check.

Regarding (1) - with last commit fixed state check in state model, update is now possible also if maintenance is pending.

@potiuk
Copy link
Member

potiuk commented Sep 12, 2025

This last error was about "flask_limiter" error. It's fixed in main now. We should rebase it I think.

@potiuk potiuk force-pushed the feature/add-maintenance-comment-change-to-edge-react-ui branch from bbe5ac8 to c6dafce Compare September 12, 2025 20:21
@jscheffl jscheffl force-pushed the feature/add-maintenance-comment-change-to-edge-react-ui branch from c6dafce to 90edd5f Compare September 12, 2025 20:29
@jscheffl
Copy link
Contributor Author

jscheffl commented Sep 12, 2025

@jscheffl for 1. I was suggesting we remove the button altogether, no need to have it until the worker enters Maintenance mode

@dheerajturaga It can sometime take a long time until worker is really in maintenance, if a longer running job need to drain. We had it multiple times that we needed to update the comment if the node is still in draining. Therefore I'd not limit the user to update comment.

Copy link
Member

@dheerajturaga dheerajturaga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Works great!

@jscheffl jscheffl force-pushed the feature/add-maintenance-comment-change-to-edge-react-ui branch from 90edd5f to 625fb94 Compare September 13, 2025 07:39
Copy link
Member

@gopidesupavan gopidesupavan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving , not an UI expert :)

@jscheffl jscheffl force-pushed the feature/add-maintenance-comment-change-to-edge-react-ui branch from 625fb94 to 1399d5f Compare September 13, 2025 11:58
@jscheffl jscheffl merged commit 2012dd1 into apache:main Sep 13, 2025
74 checks passed
suman-himanshu pushed a commit to suman-himanshu/airflow that referenced this pull request Sep 17, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
Brunda10 pushed a commit to Brunda10/airflow that referenced this pull request Sep 17, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Sep 30, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 1, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 2, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 3, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 4, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 5, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 5, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 7, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 8, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 9, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 10, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 11, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 12, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 14, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 15, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 17, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 19, 2025
* Add Worker Maintenance Comment Change to React UI

* Review proposal to fix

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>

* Update assets after change

* Fix state model, allow update of comment also in pending

---------

Co-authored-by: Dheeraj Turaga <dheerajturaga@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:edge Edge Executor / Worker (AIP-69) / edge3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants